Out of curiosity: do other clients use these shorthand names? I'm wondering if we should just use the predicate URLs directly as choices here -- that makes the CLI slightly less ergonomic, but avoids adding one more custom convention 🙂

Yes, I got these from cosign:

    --type='custom':
        specify a predicate type
        (slsaprovenance|slsaprovenance02|slsaprovenance1|link|spdx|spdxjson|cyclonedx|vuln|openvex|custom)
        or an URI

Ah my bad, I forgot I had changed them. I changed them to slsaprovenance0_2 and slsaprovenance1_0 because slsaprovenance1 seems ambiguous. But yeah, these are not standard in any way. We can switch to the URLs

Great! Makes perfect sense to use them, then.

Edit: Or maybe not, given that the current names are somewhat ambiguous. Let's go with URLs only for now, and we can always move to support shorthand names once clients unify on them.

CC @loosebazooka @haydentherapper for visibility -- this is maybe too much of an implementation detail to standardize, but something for other clients to be aware of 🙂

fixed!

Seems reasonable to me to specify the predicate type as its full URL and not deal with shorthand.

Given that this is a brand new command, I'm kind of tempted to remove/avoid the need to do anything except emit bundles 🙂 -- I think the only options we need here are --bundle and --overwrite.

The advantage of having these is that it allows us to reuse most of the code that was already written for sigstore sign. But I can remove them and refactor things around a bit.

I think this still needs to be done -- IMO it's okay if end up being able to create a reusable _sign_common, since these extra flags are about the same size as the helper anyays (and we don't want to support them).

Fixed! See last commit

Nitpick: Rather than having to explicitly declare each alias here and below, maybe we can define a common base model with a model_config that tweaks alias generation?

Something like:

from pydantic.config import ConfigDict
from pydantic.alias_generators import to_camel

class _PredicateBase(BaseModel):
    model_config = ConfigDict(alias_generator=to_camel)

class ConfigSource(_PredicateBase):
    ...

...I think something like that will work, as will save us some manual typing 🙂

Good idea! I also fixed a couple of bugs I found:

• slsa-framework/slsa-github-generator incorrectly generates predicates using buildInvocationID (note the final uppercase D, should be buildInvocationId instead). In order to fix that, I added a validation alias that allows for both options. The serialization alias will use the camelCase one set by the config you suggested.
• I was dumping the model without specifying it should output the camelCase aliases rather than our snake_case field names. Fixed by passing by_alias=True to model_dump()
• Our model dump also included the optional fields that were set to None. However, that's how we model them, those fields are just absent in the source predicate. So now we pass exclude_none=True to model_dump, to exclude those fields from the dump.

Mmm given the last point, I wonder if instead of dumping our Predicate model, we should instead just validate it and then use the exact input the user gave us to build the in-toto statement.
Otherwise we might end up modifying it when doing the JSON -> Model -> JSON roundtrip (for example, the original JSON uses ID but the final JSON uses Id).

@woodruffw WDYT about this?

@facutuesca I think that's a good idea! In principle the each JSON layout shouldn't need to be stable, but in practice users might expect that (and as you've pointed out it avoids a potential source of ser/de errors on our side).

Thinking out loud: we should probably add some documentation about when users might want to do this versus "normal" signing. But we can do that in a follow-up 🙂

Flagging for a follow-up: maybe we can make this an enum.Enum instead? That will reduce the number of unique imports we need to do, and allow us to define "supported" as "present in the enum".